Skip to content

Conversation

elvessilvavieira
Copy link
Contributor

This PR addresses #58937 by introducing support for user-defined color customization in the test runner output.

Users can now configure the theme colors by setting the NODE_TEST_RUNNER_THEME environment variable with a JSON string.

I followed the current implementation style by using the colors already defined in internal/util/colors. While the issue raises a concern about color contrast, I preferred to stick with the existing set of colors to maintain consistency. I simply chose to allow color overrides instead of changing how shouldColorize or the color system itself behaves.

I’d appreciate feedback on whether this was the right approach. Thx

image

NODE_TEST_RUNNER_THEME='{"fail":"blue","pass":"yellow","info":"red","base":"red","duration":"green","skip":"blue"}

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 6, 2025
@pmarchini
Copy link
Member

Hey @elvessilvavieira, thanks for the contribution!

Honestly, I don't think we should go down this road for the following reasons:

  1. We usually pass options via flags instead of env vars, and I don't think this case should break the standard.
  2. I don't like the idea of passing a JSON string via an option.

A user can just decide to use a custom reporter if they want a different output style.
The pain point I see is specifically related to the coverage report because, as far as I recall, it doesn't allow any kind of customization via reporter.

I still see value in what you're proposing, and I'm wondering if it could make sense to have a method like https://nodejs.org/api/test.html#assertregistername-fn that allows the user to configure its preferences.

WDYT?

@elvessilvavieira
Copy link
Contributor Author

Thanks a lot @pmarchini for the feedback

So, following your suggestion and the assert.register approach. Do you recommend something like this?

import { configureTheme } from 'node:test';

configureTheme({
  info: 'blue',
  pass: 'green',
});
 

@pmarchini
Copy link
Member

I'm not 100% sure about the function name, but yes, I was thinking about a similar API! 🚀

@elvessilvavieira
Copy link
Contributor Author

I'm not 100% sure about the function name, but yes, I was thinking about a similar API! 🚀

Thanks again. I'll implement it

@MoLow
Copy link
Member

MoLow commented Jul 7, 2025

FWIW I am -1, I think this use case doesn't really justify support in core and it can be solved with a custom reporter
not blocking tough since I am just trying to get back from a long time off from the project

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants